Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-arch build layout prep patches #590

Merged
merged 8 commits into from
Jul 3, 2019

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jul 2, 2019

Split out of #580. See individual commit messages for details.

@lucab
Copy link
Contributor

lucab commented Jul 3, 2019

basearch injection seems sane to me. I assume we don't plan to cross-build (host != target) for now.

Copy link
Contributor

@ajeddeloh ajeddeloh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM aside from one "take it or leave it" nit.

src/cmd-buildupload Show resolved Hide resolved
Add some helper functions for retrieving the dnf value of `${basearch}`.

Note there are many places over the codebase where we use `uname -m` or
`arch`, but we semantically mean `${basearch}`. I didn't bother
replacing them because (1) for the architectures we care about, it
actually makes no difference (if e.g. one day we want to support 32-bit
ARM or x86, then it will) and (2) given the above, the refactor risk
didn't seem worth it.
Mark the OSTree commit with the actual `${basearch}` we used to assemble
it. The major user for this will be Zincati. Putting it in the commit
metadata means this will then show up in e.g. `rpm-ostree status
--json`, which Zincati already executes today.

We could also do this directly in rpm-ostree. Though really, after
digging into this quite a bit, I lean towards making this a CoreOS-ism
for the time being. There aren't many software out there that care about
the specific `basearch` string; it's something that mostly only matters
to yum/dnf/rpm-ostree (hence why it's not really exposed in any easy
way).

As one can guess, any portable software that needs arch-specific
behaviours would use e.g. compiler flags or `uname(2)`, rather than any
kind of distro-specific concept.

I also didn't make this part of `/etc/os-release` because if Fedora does
end up shipping it there as well eventually, we'll have to carry *two*
variants of the same tag. OTOH, keeping it in the commit metadata forces
the few classes of software that care about this to be specific to
rpm-ostree systems, over which we likely have more control.
The default loader is not considered safe. Use `safe_load`, which
disables some of the more funky YAML features we don't need.
This is a common pattern to signal that anything that subclasses us is
expected to implement these methods.
This is useful for hacking on the codebase, but also in a devel pipeline
context.
Otherwise, an ill-timed `SIGINT` can interrupt `createrepo_c` while it's
creating the yumrepo and a follow-up invocation will barf on `.repodata`
already existing.
Minor follow-ups from the OSTree-in-builddir work.

- prune_builds: we don't need to remember the `ostree_timestamp` anymore
  when collecting builds.
- prune_builds: drop the `subprocess` module, since we no longer need to
  call out to the `ostree` CLI either.
- cmd-clean: drop the reference to OSTree repo objects
Use `builds_dir` instead of `build_dir` to refer to `builds/`, and
`build_dir` instead of `build_root` to refer to `builds/$id/`. This
matches the convention used in the rest of the codebase.
@jlebon
Copy link
Member Author

jlebon commented Jul 3, 2019

And rebased!

@jlebon jlebon merged commit f583a0d into coreos:master Jul 3, 2019
@jlebon jlebon deleted the pr/build-layout-prep branch July 6, 2020 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants